Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable big-endian simd in swap_nonoverlapping_bytes #43159

Merged
merged 1 commit into from
Jul 15, 2017

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Jul 11, 2017

This is a workaround for #42778, which was git-bisected to #40454's
optimizations to mem::swap, later moved to ptr in #42819. Natively
compiled rustc couldn't even compile stage1 libcore on powerpc64 and
s390x, but they work fine without this repr(simd). Since powerpc64le
works OK, it seems probably related to being big-endian.

The underlying problem is not yet known, but this at least makes those
architectures functional again in the meantime.

cc @arielb1

This is a workaround for rust-lang#42778, which was git-bisected to rust-lang#40454's
optimizations to `mem::swap`, later moved to `ptr` in rust-lang#42819.  Natively
compiled rustc couldn't even compile stage1 libcore on powerpc64 and
s390x, but they work fine without this `repr(simd)`.  Since powerpc64le
works OK, it seems probably related to being big-endian.

The underlying problem is not yet known, but this at least makes those
architectures functional again in the meantime.

cc @arielb1
@arielb1
Copy link
Contributor

arielb1 commented Jul 11, 2017

Nice find! I'm not sure why repr(simd) would break things so horribly - it should not have a "semantic" effect by itself. If you can come up with a "minified" example in which swap breaks that would be amazing. There might be a bug with our ABI code that is worth finding?

Maybe I'll try to look at the LLVM IR differences - I don't think there should be any.

@arielb1
Copy link
Contributor

arielb1 commented Jul 11, 2017

I'll give this an r+ for now to get things working. I am not sure on that swap PR on non-x86 architectures anyway.

@bors r+

@bors
Copy link
Contributor

bors commented Jul 11, 2017

📌 Commit 77bd4dc has been approved by arielb1

@arielb1
Copy link
Contributor

arielb1 commented Jul 11, 2017

BTW, the "broken" swap seemed to work on a ppc64 qemu (on which building the compiler failed). It would be really nice if we could find out the brokenness.

@arielb1 arielb1 added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 11, 2017
@cuviper
Copy link
Member Author

cuviper commented Jul 11, 2017

Yeah, I'm not satisfied with this long-term -- just wanted to make sure to get something in place before the next beta branch. I've tried guessing a few minimal scenarios and not seen any difference in codegen, so we'll have to actually track down which parts are different in the full build.

@bors
Copy link
Contributor

bors commented Jul 12, 2017

⌛ Testing commit 77bd4dc with merge b9cde66ebe43aa342de8979dbc3e800e6ba3b75c...

@bors
Copy link
Contributor

bors commented Jul 12, 2017

💔 Test failed - status-travis

@Mark-Simulacrum
Copy link
Member

@bors retry -- android spurious failure, hopefully fixed by clearing caches (I think this started too late, possibly)

@Mark-Simulacrum
Copy link
Member

@bors rollup

@shepmaster
Copy link
Member

Restarted the tests - looks like we ran out of disk space.

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jul 15, 2017
Disable big-endian simd in swap_nonoverlapping_bytes

This is a workaround for rust-lang#42778, which was git-bisected to rust-lang#40454's
optimizations to `mem::swap`, later moved to `ptr` in rust-lang#42819.  Natively
compiled rustc couldn't even compile stage1 libcore on powerpc64 and
s390x, but they work fine without this `repr(simd)`.  Since powerpc64le
works OK, it seems probably related to being big-endian.

The underlying problem is not yet known, but this at least makes those
architectures functional again in the meantime.

cc @arielb1
bors added a commit that referenced this pull request Jul 15, 2017
Rollup of 8 pull requests

- Successful merges: #43074, #43145, #43159, #43202, #43222, #43228, #43229, #43240
- Failed merges:
@bors bors merged commit 77bd4dc into rust-lang:master Jul 15, 2017
@cuviper cuviper deleted the ptr-swap-simd branch September 26, 2017 06:40
bors added a commit that referenced this pull request May 10, 2019
Revert "Disable big-endian simd in swap_nonoverlapping_bytes"

This reverts commit 77bd4dc (#43159).

Issue #42778 was formerly easy to reproduce on two big-endian targets,
`powerpc64` and `s390x`, so we disabled SIMD on this function for all
big-endian targets as a workaround.

I have re-tested this code on `powerpc64` and `s390x`, each with the
bundled LLVM 8 and with external LLVM 7 and LLVM 6, and the problems no
longer appear. So it seems safe to remove this workaround, although I'm
still a little uncomfortable that we never found a root-cause...

Closes #42778.
r? @arielb1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants